-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CsvEncoder #486
Add CsvEncoder #486
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should call out metafacture-csv-plugin as the "inspiration" (to put it mildly). And also deprecate the plugin as a consequence of being merged into core.
metafacture-csv/src/main/java/org/metafacture/csv/CsvEncoder.java
Outdated
Show resolved
Hide resolved
metafacture-csv/src/main/java/org/metafacture/csv/CsvEncoder.java
Outdated
Show resolved
Hide resolved
metafacture-csv/src/main/java/org/metafacture/csv/CsvEncoder.java
Outdated
Show resolved
Hide resolved
metafacture-csv/src/test/java/org/metafacture/csv/CsvEncoderTest.java
Outdated
Show resolved
Hide resolved
Right. It's a copy. I thought to add @eberhardtj as author but I thought the author is hijacked resp. not the original one, because it's somehow ghosted (https://github.com/metafacture/metafacture-csv-plugin) and her github history starts 2019 and it's spares. So I mentioned at least the dnb as first contributor.
+1 doing this when merged. |
Yes, I would probably just use the user name without the
Yes, sounds good. If it were me I would also reference the original repo, so one can follow the Git history. |
Copied from https://github.com/metafacture/metafacture-csv-plugin. Original author: eberhardtj ([email protected]).
Co-authored-by: Jens Wille <[email protected]>
Co-authored-by: Jens Wille <[email protected]>
Co-authored-by: Jens Wille <[email protected]>
Reworked PR. Please have a look again @blackwinter . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the unit test from my previous comment in f3b3d92.
You could replace the setup()
method with a MockitoRule
, but that's up to you.
metafacture-csv/src/test/java/org/metafacture/csv/CsvEncoderTest.java
Outdated
Show resolved
Hide resolved
metafacture-csv/src/test/java/org/metafacture/csv/CsvEncoderTest.java
Outdated
Show resolved
Hide resolved
Where is the documentation in https://github.com/metafacture/metafacture-documentation/blob/master/flux-commands.md |
The |
ah - sorry! Amending the commits and force pushing must have lost these. Good that you checked that :) |
I tested without options and |
Done this, see https://github.com/metafacture/metafacture-csv-plugin/blob/master/README.adoc. |
See #483 .